feat(cli_tools): Enable IP geotracking in analytics events #93
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughMixPanelAnalytics constructor was changed: Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant MixPanelAnalytics
participant HTTP
Caller->>MixPanelAnalytics: new MixPanelAnalytics(..., endpoint?, disableIpTracking?)
Note right of MixPanelAnalytics: constructor
MixPanelAnalytics->>MixPanelAnalytics: _buildEndpoint(baseEndpoint, disableIpTracking)\n(parse, preserve queries, set ip=0/1)
MixPanelAnalytics->>MixPanelAnalytics: store `_endpoint` as Uri
Caller->>MixPanelAnalytics: trackEvent(...)
MixPanelAnalytics->>MixPanelAnalytics: _quietPost(payload)
MixPanelAnalytics->>HTTP: POST `_endpoint` with payload
HTTP-->>MixPanelAnalytics: response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli_tools/lib/src/analytics/analytics.dart (1)
34-34: Consider using positive boolean naming for clarity.The parameter
disableIpTrackinguses negative naming, which can lead to double-negative logic (e.g.,!disableIpTrackingto check if enabled). Consider renaming toenableIpTrackingwith a default oftruefor improved readability.If you prefer positive naming, apply this diff:
- final bool disableIpTracking = false, + final bool enableIpTracking = true, }) : _uniqueUserId = uniqueUserId, _projectToken = projectToken, _version = version, _endpoint = (endpoint ?? _defaultEndpoint) + - (disableIpTracking ? '?ip=0' : '?ip=1'), + (enableIpTracking ? '?ip=1' : '?ip=0'), _timeout = timeout;Note: This would need to be combined with the query string fix from the previous comment.
Also applies to: 38-39
SandPod
left a comment
SandPod
left a comment
There was a problem hiding this comment.
We don't have any custom parameters in Serverpod, so the CodeRabbit issue is not critical for us.
Although it would probably be good to use that approach (uri.replace) to support any additional parameters.
To enable IP-based geotracking of the client origin, MixPanel requires the URI query parameter
ipto be set to 1.Summary by CodeRabbit
New Features
Improvements